Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MRG] Tensorflow backend & Benchmarker & Myst_parser #316

Merged
merged 48 commits into from
Dec 9, 2021

Conversation

ncassereau
Copy link
Contributor

@ncassereau ncassereau commented Nov 29, 2021

Personal To do

  • Prettify benchmark results presentation
  • Include benchmark for EMD
  • Correct time measure for each backend (does time.perf_counter work on GPUs ??)
  • Make real benchmark for Sinkhorn Knopp that is pretty for this PR (with every single backend and device)
  • Put aforementioned benchmark in backend.py's docstring
  • Check if set_gradient works as intended for tensorflow and add test
  • Add to backend.py docstrings a comment about tensorflow requiring the numpy API
  • Replace m2r2 by myst_parser (https://myst-parser.readthedocs.io/en/latest/)

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Motivation and context / Related issue

  • Backend for tensorflow.
  • Also adds a benchmark folder which allows to easily benchmark every backend and present results in a html table (compatible with github comments and PR text).
  • Also replaced m2r2 by myst_parser. It allows to use markdown files in our official docs. Myst_parser seems more maintained. It also happens to solve the issue with internal links introduced by the m2r2 update few PRs ago.

Note that tensorflow does not know how to transpose inplace, which can have a negative impact on performance for any function which requires the usage of a transpose (More information here: https://www.tensorflow.org/api_docs/python/tf/transpose#expandable-1)

Note that nothing was done regarding the device consistency of our calculation (unlike TorchBackend for instance). This is due to the fact that tensorflow chooses the device automatically without caring about the device where inputs are located. If the user wants a specific device, this should be done outside of POT, probably something like

with tf.device("/GPU:1"):
    ot.sinkhorn(args, **kwargs)

More information here: https://www.tensorflow.org/guide/gpu#overview

How has this been tested (if it applies)

Tested with Tensorflow 2.6.0 and 2.7.0. Benchmark with Tensorflow 2.6.0

Performance on Sinkhorn Knopp

The speedup is not as convincing as with cupy, might be due to how inefficient tensorflow transpose are. Still a great choice for very high dimensions.
Used pretty much the code from test/test_gpu.py::test_gpu_sinkhorn.
Tests were performed with Tensorflow 2.6.0, results in seconds, averaged over 100 runs.

Sinkhorn Knopp - Averaged on 100 runs
Bitsize32 bits
DeviceCPUGPU
Sample sizeNumpyPytorchTensorflowCupyJaxPytorchTensorflow
500.00080.00220.01510.00950.01930.00510.0293
1000.00050.00130.00970.00570.01150.00290.0173
5000.00090.00160.01100.00580.01150.00290.0166
10000.00210.00210.01450.00560.01180.00290.0168
20000.00690.00430.02780.00590.01180.00300.0165
50000.07070.03140.13950.00740.01250.00350.0198
 
Bitsize64 bits
DeviceCPUGPU
Sample sizeNumpyPytorchTensorflowCupyJaxPytorchTensorflow
500.00080.00200.01540.00930.01910.00510.0328
1000.00050.00130.00940.00560.01140.00290.0169
5000.00130.00170.01200.00590.01160.00290.0168
10000.00340.00270.01770.00580.01180.00290.0167
20000.01460.00750.04360.00590.01200.00290.0165
50000.14670.05680.24680.00770.01460.00450.0204
Code used for the test
python3 -m benchmarks.sinkhorn_knopp

Checklist

  • The documentation is up-to-date with the changes I made.
  • I have read the CONTRIBUTING document.
  • All tests passed, and additional code has been covered with new tests.

@codecov
Copy link

codecov bot commented Nov 30, 2021

Codecov Report

Merging #316 (86faaa4) into master (b3dc68f) will decrease coverage by 0.31%.
The diff coverage is 88.75%.

@@            Coverage Diff             @@
##           master     #316      +/-   ##
==========================================
- Coverage   93.34%   93.03%   -0.32%     
==========================================
  Files          21       21              
  Lines        4901     5198     +297     
==========================================
+ Hits         4575     4836     +261     
- Misses        326      362      +36     

@ncassereau ncassereau changed the title [WIP] Tensorflow backend [MRG] Tensorflow backend Dec 3, 2021
@ncassereau ncassereau marked this pull request as ready for review December 3, 2021 15:26
@ncassereau ncassereau changed the title [MRG] Tensorflow backend [MRG] Tensorflow backend & Benchmarker Dec 7, 2021
@ncassereau ncassereau marked this pull request as draft December 7, 2021 10:28
@ncassereau ncassereau changed the title [MRG] Tensorflow backend & Benchmarker [WIP] Tensorflow backend & Benchmarker Dec 7, 2021
@ncassereau ncassereau changed the title [WIP] Tensorflow backend & Benchmarker [MRG] Tensorflow backend & Benchmarker Dec 7, 2021
@ncassereau ncassereau marked this pull request as ready for review December 7, 2021 14:07
@rflamary
Copy link
Collaborator

rflamary commented Dec 7, 2021

This is awesome, thank you so much @ncassereau-idris

@ncassereau ncassereau changed the title [MRG] Tensorflow backend & Benchmarker [MRG] Tensorflow backend & Benchmarker & Myst_parser Dec 9, 2021
@rflamary rflamary merged commit f8d871e into PythonOT:master Dec 9, 2021
@ncassereau ncassereau deleted the tensorflow_backend branch December 10, 2021 08:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants